[Storage-Queue][Storage-File] Kwargify positional params by rakshith91 · Pull Request #7877 · Azure/azure-sdk-for-python

rakshith91 · 2019-10-14T21:27:42Z

adxsdk6 · 2019-10-14T21:27:44Z

Can one of the admins verify this patch?

kristapratico · 2019-10-15T17:14:45Z

        ):
        # type: (...) -> None
        kwargs['retry_policy'] = kwargs.get('retry_policy') or ExponentialRetry(**kwargs)
+        loop = kwargs.pop('loop', None)


Add :keyword loop: to docstring?

kristapratico · 2019-10-15T17:17:17Z

    @distributed_trace
-    def list_handles(self, recursive=False, timeout=None, **kwargs):
+    def list_handles(self, recursive=False, **kwargs):
        # type: (bool, Optional[int], Any) -> AsyncItemPaged


We can remove the Optional[int] in the type hint.

kristapratico · 2019-10-15T17:18:07Z

    @distributed_trace_async
-    async def set_directory_metadata(self, metadata, timeout=None, **kwargs): # type: ignore
+    async def set_directory_metadata(self, metadata, **kwargs): # type: ignore
        # type: (Dict[str, Any], Optional[int], Any) ->  Dict[str, Any]


minus Optional[int]

kristapratico · 2019-10-15T17:20:18Z

            Maximum number of parallel connections to use.
-        :param int timeout:
+        :keyword int timeout:
            The timeout parameter is expressed in seconds.


Need to change metadata, content_settings, validate_content, max_concurrency, and encoding to :keyword: as well.

kristapratico · 2019-10-15T17:21:35Z

            The name of the file to delete.
-        :param int timeout:
+        :keyword int timeout:
            The timeout parameter is expressed in seconds.


Looks like we're keeping this timeout as a param.

On second thought, was this one just missed being put into kwargs?

kristapratico · 2019-10-15T17:22:18Z

        snapshot=None,  # type: Optional[Union[str, Dict[str, Any]]]
        credential=None,  # type: Optional[Any]
-        loop=None,  # type: Any
        **kwargs  # type: Any


keyword loop in docstring?

kristapratico · 2019-10-15T17:23:53Z

        :type copy_id: str or ~azure.storage.file.FileProperties
        :rtype: None
        """
+        timeout = kwargs.pop('timeout', None)


:keyword int timeout: not in docstring

kristapratico · 2019-10-15T17:25:46Z

    def __init__(
            self, account_url,  # type: str
            credential=None,  # type: Optional[Any]
-            loop=None,  # type: Any


loop docstring

kristapratico · 2019-10-15T17:27:54Z

    @distributed_trace_async
-    async def create_directory(self, directory_name, metadata=None, timeout=None, **kwargs):
+    async def create_directory(self, directory_name, **kwargs):
        # type: (str, Optional[Dict[str, Any]], Optional[int], Any) -> DirectoryClient


we can remove Optional[int] in type hint

kristapratico · 2019-10-15T17:36:59Z

            The timeout parameter is expressed in seconds.
        '''
-
+        timeout = kwargs.pop('timeout', None)


timeout isn't being passed to any method

xiangyan99

Please double check the places we use the kwargified params. Seems like we can simplify the code.

xiangyan99 · 2019-10-15T18:38:15Z

        share = self.get_share_client(share_name)
        kwargs.setdefault('merge_span', True)
-        await share.create_share(metadata, quota, timeout, **kwargs)
+        await share.create_share(metadata=metadata, quota=quota, timeout=timeout, **kwargs)


Why not just **kwargs?

xiangyan99 · 2019-10-15T18:38:47Z

        """
+        timeout = kwargs.pop('timeout', None)
        try:
            return await self._client.service.get_properties(timeout=timeout, **kwargs)


Why not just **kwargs?

xiangyan99 · 2019-10-15T18:40:06Z

+        metadata = kwargs.pop('metadata', None)
+        timeout = kwargs.pop('timeout', None)
        subdir = self.get_subdirectory_client(directory_name)
        subdir.create_directory(metadata=metadata, timeout=timeout, **kwargs)


Updated everywhere, and history.md

rakshith91 · 2019-10-15T22:51:44Z

CI is green
https://dev.azure.com/azure-sdk/public/_build/results?buildId=146882&view=results

* Fix SubStream to respect IOBase protocol (#7843) * removes NoRetry policy (#7845) * [storage] Makes signed_identifiers a required param (#7844) * makes signed_identifiers a required param * fix tests and update history * removes unecessary check * fix files and queues tests * [storage] Changes `file_permission_key` param to `permission_key` (#7841) * changes file_permission_key param to permission_key * update history * Change the param directive to keyword in docstrings (#7855) * Change the directive to keyword in docstrings * pylint :( * [storage] Adds from_string to models (#7870) * adds from_string to models * fixes to naming and history * remove _str in docstrings * pylint * [storage-file] await async poller (#7872) * await files async poller * fix docstrings * fix type hints * [File and Queue] Client constructors (#7853) * from_queue_url * file client constructors * update CHANGELOG * fix test * lint fix * some doc changes * queue docs * blob docs * [storage-queue] rename queue messages (#7895) * rename queue messages * update history * [Storage-Queue][Storage-File] Kwargify positional params (#7877) * Queue kwargification * kwargify Files * small fix * minor fix * comments + lint * some changes * pylint * [storage-file, queue] moves param protocol to kwargs in gen_shared_access_signature() (#7897) * moves protocol in gen_sas to kwargs * delete whitespace * [storage] Unexposes models from aio (#7881) * unexposes models from aio * unexpose all except clients and pylint fixes * update history * unexpose sync paged models * update history phrasing * [Storage-queue] Allow None message encode policy (#7898) * Don't expose NoEncode policies * Support None value for encode/decode policies * Updated tests * Updated sync encryption tests * Updated async encryption tests * Updated async encoding tests * Fixed test * Fix test * Updated release notes * Rename enqueue_message to send_messgae (#7928) * [storage-blob, queue] Renames Logging to <ServiceName>AnalyticsLogging (#7921) * renames logging to ServiceAnalyticsLogging * changes param logging to analytics_logging * fixes tests due to param name change * [Blob][File][Encryption]Fix Download Encrypted Blob/File Bug (#7965) #7957 * Consolidate offset - length behavior in files (#7942) * Initital Commit * length changes * some test changes * some more changes * history.md * Update a couple of recrodings * comments address * ops * Stop modifying internal_response body (#7958) * Stop modifying internal body * Replicate for queues and files too * [Storage] Relocated SAS generation (#7955) * Expose account name on clients * Updated blob sas gen * Updated file sas gen * Update queue sas gen * Updated docstrings * Some pylint fixes * More pylint * More pylint * Reverted auth change * Added release notes * Fixed kwarg docstring * [Storage] Updated close handles (#7940) * Updated close handles * Pylint fixes * Fix tests * Review feedback * Missing recursive parameter * Added release notes * [Storage] Conditional etag parameters (#8047) * Updated clients * Process conditional headers * Updated tests * Fixed test * Updated error scenarios * Updated release notes * Revert skipped tests * [Storage] Download stream refactor (#7848) * Refactored download stream API * Unskip tests * Test warnings * Missing await * Fixed append tests * Fixed page tests * Pylint * Py2.7 iter support * Refactor downloaders * Added missing async module functions * Updated release notes * Keyword params * Documented more keyword options * Added module functions to exports * Fixed merge * Fix for Files download stream * Pylint fix * Updated File stream downloads * Fix tests * Updated error message * Added Files release notes * Pylint fix * Download stream error handling * Fixed some tests * Design Pipeline ownership (#7981) * Initital Commit * Pipeline Ownership * slight modifications * pipeline ownership for queues and files * fixes tests, adds docstrings to wrapper classes * pylint * Batching APIs Raise on Any Failure (#7963) * Raise on Single Failure * some changes * lint * some changes * oops * comments + lint * some optimization * history * comments * lint * recording * Plug HttpLoggingPolicy to Storage (#8081) * Plug HttpLoggingPolicy to Storage * Update dependencies * Skip depends job for Storage * Fix shared req * [storage] make storage files _internal (#7949) * blobs internal * files internal * queues internal * some fixes to docstrings * update history.md * fix async test import * some file mypy fixes * some queue mypy fixes * some blob mypy fixes * fix mistake in type annot * history edits * Merge fix * Revert "Merge fix" This reverts commit 748fbd6. * Better merge fix * fix some tests (#8100) * Fix tests (#8103) * minor fix special char * fix auth * comments * [Storage] Bumped version + Pipeline fix (#8089) * Bumped version * Docs tweak * Re-bumped version * Added 3.8 classifier * Added proxy policy to pipeline * Synced base clients * Added batch exception * Comment the echo check to workaround batch headers issue (#8118) * Fix 8091: expose generated enum to customers (#8117) * Fix #8091 * Add one in file

Queue kwargification

f4a5aeb

rakshith91 added P0 Storage Storage Service (Queues, Blobs, Files) Client This issue points to a problem in the data-plane of the library. blocking-release Blocks release labels Oct 14, 2019

rakshith91 self-assigned this Oct 14, 2019

kwargify Files

f393688

rakshith91 marked this pull request as ready for review October 15, 2019 16:17

rakshith91 requested review from annatisch and zezha-msft as code owners October 15, 2019 16:17

rakshith91 added 2 commits October 15, 2019 09:19

small fix

efeebdd

minor fix

6bc6c2b

rakshith91 changed the title ~~[Storage-Queue] Kwargify positional params in storage queues~~ [Storage-Queue][Storage-File] Kwargify positional params in storage queues Oct 15, 2019

rakshith91 changed the title ~~[Storage-Queue][Storage-File] Kwargify positional params in storage queues~~ [Storage-Queue][Storage-File] Kwargify positional params Oct 15, 2019

kristapratico reviewed Oct 15, 2019

View reviewed changes

kristapratico approved these changes Oct 15, 2019

View reviewed changes

rakshith91 added 2 commits October 15, 2019 10:53

comments + lint

954e3c5

Merge branch 'feature/storage-preview5' into keywrordqueuefile

171ee84

xiangyan99 reviewed Oct 15, 2019

View reviewed changes

some changes

e76ff0e

xiangyan99 approved these changes Oct 15, 2019

View reviewed changes

rakshith91 added 2 commits October 15, 2019 13:23

Merge branch 'feature/storage-preview5' into keywrordqueuefile

d559601

pylint

a9d9b64

rakshith91 merged commit 72a064c into Azure:feature/storage-preview5 Oct 15, 2019

rakshith91 deleted the keywrordqueuefile branch October 15, 2019 22:51

Uh oh!

Conversation

rakshith91 commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adxsdk6 commented Oct 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiangyan99 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rakshith91 commented Oct 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rakshith91 commented Oct 14, 2019 •

edited

Loading